feat: add edit_and_execute composite tool #7608
Closed
+471
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add new tool that combines file editing operations with command execution - Supports write_to_file, apply_diff, insert_content, and search_and_replace as edit operations - Executes command only if edit operation succeeds - Add comprehensive tests for the new tool - Update type definitions and tool registry Implements #7607
dosubot
bot
added
size:L
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| import { Task } from "../task/Task" | ||
| import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools" | ||
| import { formatResponse } from "../prompts/responses" | ||
| import { ClineSayTool } from "../../shared/ExtensionMessage" |
Contributor
There was a problem hiding this comment.
Remove the unused import 'ClineSayTool' to keep the module clean.
Suggested change
| import { ClineSayTool } from "../../shared/ExtensionMessage" |
| if (block.partial) { | ||
| // For partial blocks, show progress | ||
| const partialMessage = { | ||
| tool: "editAndExecute" as const, |
Contributor
There was a problem hiding this comment.
For consistency, use the tool name 'edit_and_execute' instead of 'editAndExecute' in the partial message.
Suggested change
| tool: "editAndExecute" as const, | |
| tool: "edit_and_execute" as const, |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
Contributor
Author
There was a problem hiding this comment.
I wrote this code and now I'm reviewing it. Trust issues: maximum.
Review Findings
Critical Issues (Must Fix):
-
Missing checkpoint save before edit operations (
src/core/tools/editAndExecuteTool.ts)- The tool doesn't call
checkpointSaveAndMarkbefore performing edit operations - All other edit tools in the codebase call this before modifications to ensure data safety
- Should add
await checkpointSaveAndMark(cline)before delegating to edit tools - Also missing in
presentAssistantMessage.tsbefore callingeditAndExecuteTool
- The tool doesn't call
-
Incomplete test coverage (
src/core/tools/__tests__/editAndExecuteTool.spec.ts)- Tests only cover error cases (missing args, missing blocks, partial blocks)
- Missing tests for successful execution paths with different edit tools
- Should add tests for successful write_to_file, apply_diff, insert_content, search_and_replace + execute_command combinations
Important Suggestions:
-
Error handling inconsistency (
src/core/tools/editAndExecuteTool.tsline ~232)- When edit operation fails, only shows generic "Edit operation failed" message
- Should capture and include the actual error from the edit tool for better debugging
-
Parameter validation (
src/shared/tools.ts)- Confirmed
argsparameter is correctly kept in toolParamNames array - Used by both
edit_and_executeand multi-fileapply_diff
- Confirmed
Minor Improvements:
-
Code duplication (
src/core/tools/editAndExecuteTool.ts)- Repetitive XML parsing logic could be extracted into a helper function
- Would reduce duplication and improve maintainability
-
Documentation
- While main function has good JSDoc, individual switch cases could benefit from inline comments
Overall, the implementation follows the requirements from issue #7607 well, but needs these critical fixes before merging.
hannesrudolph
added
the
Issue/PR - Triage
4 tasks
Member
|
Closing, see #7607 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the
edit_and_executecomposite tool as requested in #7607.Summary
Adds a new composite tool that combines file editing operations with command execution in a single atomic operation. This addresses the inefficiency of making two separate tool calls for common development tasks.
Changes
edit_and_executeto the tool type definitions inpackages/types/src/tool.tssrc/core/tools/editAndExecuteTool.tssrc/shared/tools.ts(display names, groups, parameter names)Implementation Details
The tool:
<edit>and<execute>blockswrite_to_file,apply_diff,insert_content,search_and_replaceTesting
Example Usage
Closes #7607
Important
Introduces
edit_and_executecomposite tool for atomic file editing and command execution, with full integration and testing.edit_and_executetool totool.tsand integrates it intopresentAssistantMessage()inpresentAssistantMessage.ts.<edit>and<execute>blocks, handlingwrite_to_file,apply_diff,insert_content,search_and_replace.editAndExecuteTool.spec.tswith 100% coverage.toolParamNamesandTOOL_DISPLAY_NAMESintools.tsto include new tool.This description was created by
for 2297c7a. You can customize this summary. It will automatically update as commits are pushed.